Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support extended room names #210

Merged
merged 3 commits into from
Jan 24, 2024
Merged

Support extended room names #210

merged 3 commits into from
Jan 24, 2024

Conversation

H-Shay
Copy link
Contributor

@H-Shay H-Shay commented Jan 23, 2024

The fosdem schedule has a number of rooms with more than word, ie "K.1.105 (La Fontaine)" or "H.1302 (Depage)" and some of the commands were splitting the input args such that the second part of the name was being cut, meaning that the commands were unable to find the rooms.

This PR adds support for rooms with more than one word in them by changing how the args are parsed when the commands are run.

The other option for fixing this would be to strip all the rooms as they are parsed/stored - I considered that as well but this seemed faster - happy to change course if it's incorrect.

@H-Shay H-Shay requested a review from a team as a code owner January 23, 2024 05:01
@reivilibre
Copy link
Contributor

just as a note of interest, in previous years (or at least last year) these were 'slugified' into k1.1.05_la_fontaine etc or something like that and you'd refer to them by slug.

@H-Shay
Copy link
Contributor Author

H-Shay commented Jan 23, 2024

just as a note of interest, in previous years (or at least last year) these were 'slugified' into k1.1.05_la_fontaine etc or something like that and you'd refer to them by slug.

When the room is converted in an auditorium the name as given from the schedule is stored as id, and there is a separate field for the slug (slugified name). The function that looks up the input from the user in Verify and other commands, getAuditorium, looks up the auditorium by auditorium.id rather than auditorium.slug - I see the slug being used in the room aliases, though. So i think this change is still needed?

Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems good otherwise, thanks :-)

(though I'm not sure IDs should be multiple words TBH buuuuut if they are, they are)

src/commands/VerifyCommand.ts Outdated Show resolved Hide resolved
@H-Shay H-Shay merged commit 3deb801 into main Jan 24, 2024
5 checks passed
@H-Shay H-Shay deleted the shay/support_rooms branch January 24, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants